Skip to content

Conversation

DinoV
Copy link
Contributor

@DinoV DinoV commented Feb 10, 2025

Marks immortal objects as having the deferred reference counting bit, allowing these objects to be cached in the specializing interpreter loop.

Gets a significant perf win on deltablue where small ints are stored in a class and loaded repeatedly: https://github.com/facebookexperimental/free-threading-benchmarking/blob/main/results/bm-20250210-3.14.0a4%2B-8e67151-NOGIL/bm-20250210-vultr-x86_64-DinoV-immortal_deferred-3.14.0a4%2B-8e67151-vs-base.svg

@mpage
Copy link
Contributor

mpage commented Feb 10, 2025

I think we probably don't want to mark immortal objects as deferred and instead should allow immortal objects to be stored in inline caches (i.e. check that the object either uses deferred refcounting or is immortal)

@colesbury
Copy link
Contributor

I think this makes sense. When we are checking if an object uses deferred reference counting, we nearly always want to include immortal objects too. We can rename the flag if it improves clarity.

  • We'd probably want to set the bit in _Py_SetImmortalUntracked too.
  • We can simplify the check in
    if (_Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj)) {
    if we're consistent about it.

@DinoV DinoV force-pushed the immortal_deferred branch from 8e67151 to 1d65711 Compare February 11, 2025 00:19
@DinoV DinoV force-pushed the immortal_deferred branch 2 times, most recently from 04d3398 to 2981305 Compare February 11, 2025 17:54
@DinoV DinoV force-pushed the immortal_deferred branch from 2981305 to fd742f4 Compare February 12, 2025 00:40
@DinoV DinoV requested a review from methane as a code owner February 12, 2025 00:40
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DinoV DinoV merged commit 28f5e3d into python:main Feb 13, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants